[DNM] zephyrCommon: Improved interrupt handling #136

soburi · 2025-08-23T11:57:38Z

The structure has been revised and unnecessary fields have been removed.
Interrupt masks are now directly modified by changing the pin_mask
parameter in struct gpio_callback.
Renamed non-Arduino-derived setInterruptHandler and handleGpioCallback
functions to Zephyr-like names.

Zephyr's ba48d83b changes make it easy to configure pin numbering at compile, so we'll change the pin numbers to be based on the GPIO pin numbers. This is same as the traditional Arduino, using the GPIO numbers as pin numbers. Pin names written on the board, such as D1 and D2, are aliases for these pin numbers. When using multiple GPIO ports, pin numbers are numbered consecutively from the beginning. For example, if you have two 16-port GPIOs, 16 refers to the first pin (idx=0) of the second port. Signed-off-by: TOKITA Hiroshi <tokita.hiroshi@gmail.com>

Now that GPIO can be specified generically, we will discontinue the definition generation of LED_BUILTIN from builtin_led_gpios. Delete this definition where automatic generation from led0 is not an issue, and define it in variant.h in all other cases. Signed-off-by: TOKITA Hiroshi <tokita.hiroshi@gmail.com>

- The structure has been revised and unnecessary fields have been removed. - Interrupt masks are now directly modified by changing the pin_mask parameter in `struct gpio_callback`. - Renamed non-Arduino-derived `setInterruptHandler` and `handleGpioCallback` functions to Zephyr-like names. Signed-off-by: TOKITA Hiroshi <tokita.hiroshi@gmail.com>

Copilot

Pull request overview

This PR refactors GPIO and interrupt handling in the Zephyr Arduino core to use a more direct port-based approach. The changes eliminate the need for the builtin-led-gpios devicetree property by using variant.h definitions or led0 aliases, simplify the interrupt callback structure by directly modifying pin_mask, and rename internal functions to follow Zephyr naming conventions.

Key changes:

Replaced per-pin GPIO management with a port-based system using gpio-ports devicetree property
Streamlined interrupt handling by removing unnecessary fields from callback structure and using pin_mask directly
Renamed internal functions to snake_case (Zephyr style): setInterruptHandler → set_interrupt_handler, handleGpioCallback → handle_gpio_callback

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
variants//.overlay	Added gpio-ports property and removed builtin-led-gpios property to support new port-based GPIO management
variants/cc3220sf_launchxl_cc3220sf/variant.h	Changed LED definitions from numeric pins to D-prefixed constants for consistency
variants/arduino_nano_33_ble_*/variant.h	Added LED_BUILTIN definition and pragma once header guard
documentation/variants.md	Updated builtin-LED documentation to reflect new variant.h precedence and removed complex builtin-led-gpios configuration examples
cores/arduino/Arduino.h	Refactored pin number calculation macros from DIGITAL_PIN_* to ZARD_* family, simplified LED_BUILTIN resolution logic
cores/arduino/zephyrCommon.cpp	Major refactoring: introduced port-based GPIO helpers, simplified interrupt callback structure, renamed internal functions to Zephyr style, updated all GPIO operations to use new helpers

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-19T14:28:27Z

cores/arduino/zephyrCommon.cpp

-void handleGpioCallback(const struct device *port, struct gpio_callback *cb, uint32_t pins)
-{
-  struct gpio_port_callback *pcb = (struct gpio_port_callback *)cb;
+void handle_gpio_callback(const struct device *port, struct gpio_callback *cb, uint32_t pins) {


The port parameter is declared but never used in the function body. While this matches the required GPIO callback signature, consider using ARG_UNUSED(port) or similar to suppress potential compiler warnings about unused parameters.

Suggested change

void handle_gpio_callback(const struct device *port, struct gpio_callback *cb, uint32_t pins) {

void handle_gpio_callback(const struct device *port, struct gpio_callback *cb, uint32_t pins) {

(void)port;

Copilot · 2025-12-19T14:28:28Z

cores/arduino/zephyrCommon.cpp

 #define GPIO_NGPIOS(n, p, i) DT_PROP(DT_GPIO_CTLR_BY_IDX(n, p, i), ngpios)
-const int max_ngpios = max_in_list(
+constexpr int max_ngpios = max_in_list(
  0, DT_FOREACH_PROP_ELEM_SEP(DT_PATH(zephyr_user), digital_pin_gpios, GPIO_NGPIOS, (, )));

 /*
 * GPIO callback implementation
 */

-struct arduino_callback {
-  voidFuncPtr handler;
-  bool enabled;
-};
-
 struct gpio_port_callback {
  struct gpio_callback callback;
-  struct arduino_callback handlers[max_ngpios];
-  gpio_port_pins_t pins;
-  const struct device *dev;
-} port_callback[port_num] = {0};
+  voidFuncPtr handlers[max_ngpios];
+} port_callback[ARRAY_SIZE(gpios)];


The handlers array is sized using max_ngpios which is calculated from digital_pin_gpios. However, the array is indexed by local_gpio_pin which operates on gpio_ports. If gpio_ports contains a port with more pins than the maximum found in digital_pin_gpios, this could cause an out-of-bounds access. Consider calculating max_ngpios from gpio_ports instead to ensure the array is always large enough for any pin on any configured GPIO port.

Copilot · 2025-12-19T14:28:28Z

cores/arduino/zephyrCommon.cpp

 void enableInterrupt(pin_size_t pinNumber) {
-  struct gpio_port_callback *pcb = find_gpio_port_callback(arduino_pins[pinNumber].port);
+  struct gpio_port_callback *pcb = find_gpio_port_callback(local_gpio_port(pinNumber));

  if (pcb) {
-    pcb->handlers[arduino_pins[pinNumber].pin].enabled = true;
+    pcb->callback.pin_mask |= BIT(local_gpio_pin(pinNumber));
  }


If local_gpio_pin returns pin_size_t(-1) for an invalid pin, BIT(local_gpio_pin(pinNumber)) will attempt to shift by a very large value (since -1 cast to unsigned is the maximum value), which causes undefined behavior. Consider checking if local_gpio_pin returns a valid value before using it in the BIT macro.

Copilot · 2025-12-19T14:28:28Z

cores/arduino/zephyrCommon.cpp

 void disableInterrupt(pin_size_t pinNumber) {
-  struct gpio_port_callback *pcb = find_gpio_port_callback(arduino_pins[pinNumber].port);
+  struct gpio_port_callback *pcb = find_gpio_port_callback(local_gpio_port(pinNumber));

  if (pcb) {
-    pcb->handlers[arduino_pins[pinNumber].pin].enabled = false;
+    pcb->callback.pin_mask &= ~BIT(local_gpio_pin(pinNumber));
  }


If local_gpio_pin returns pin_size_t(-1) for an invalid pin, BIT(local_gpio_pin(pinNumber)) will attempt to shift by a very large value (since -1 cast to unsigned is the maximum value), which causes undefined behavior. Consider checking if local_gpio_pin returns a valid value before using it in the BIT macro.

Copilot · 2025-12-19T14:28:28Z

documentation/variants.md

+```
+/ {
+       zephyr,user {
+               gpios = <&gpio0>, <&gpio1>;


The documentation refers to a node called "gpios" but all the overlay files use "gpio-ports" (with a hyphen). The documentation should use "gpio-ports" to match the actual property name used in the devicetree overlays.

Copilot · 2025-12-19T14:28:29Z

cores/arduino/zephyrCommon.cpp

+  struct gpio_port_callback *pcb = find_gpio_port_callback(local_gpio_port(pinNumber));



The function does not check if local_gpio_port returns nullptr before calling find_gpio_port_callback. If an invalid pinNumber is passed, local_gpio_port could return nullptr, which would then be compared against valid device pointers in find_gpio_port_callback, potentially matching nullptr in the gpios array or causing undefined behavior. Consider adding a null check before calling find_gpio_port_callback.

Suggested change

struct gpio_port_callback *pcb = find_gpio_port_callback(local_gpio_port(pinNumber));

const struct device *dev = local_gpio_port(pinNumber);

if (!dev) {

return;

}

struct gpio_port_callback *pcb = find_gpio_port_callback(dev);

Copilot · 2025-12-19T14:28:29Z

cores/arduino/zephyrCommon.cpp

+  gpio_pin_interrupt_configure(local_gpio_port(pinNumber), local_gpio_pin(pinNumber), 0);
  disableInterrupt(pinNumber);


The order of operations in detachInterrupt could lead to a race condition. The GPIO interrupt is disabled (line 411) before the pin_mask is cleared via disableInterrupt (line 412). If an interrupt fires between these two operations, handle_gpio_callback could still execute because the pin_mask hasn't been cleared yet. Consider calling disableInterrupt before gpio_pin_interrupt_configure to ensure the pin_mask is cleared first, preventing the callback from executing.

Suggested change

gpio_pin_interrupt_configure(local_gpio_port(pinNumber), local_gpio_pin(pinNumber), 0);

disableInterrupt(pinNumber);

disableInterrupt(pinNumber);

gpio_pin_interrupt_configure(local_gpio_port(pinNumber), local_gpio_pin(pinNumber), 0);

Copilot · 2025-12-19T14:28:29Z

cores/arduino/zephyrCommon.cpp

+  const struct device* port = local_gpio_port(pinNumber);
+  const size_t pin = local_gpio_pin(pinNumber);
  struct k_timer timer;
  int64_t start, end, delta = 0;
-  const struct gpio_dt_spec *spec = &arduino_pins[pinNumber];

-  if (!gpio_is_ready_dt(spec)) {
+  if (!device_is_ready(port)) {
    return 0;


The function does not check if local_gpio_port returns nullptr before passing it to device_is_ready. If an invalid pinNumber is provided, this could cause undefined behavior. Consider adding an explicit null check for port before using it.

Copilot · 2025-12-19T14:28:30Z

cores/arduino/zephyrCommon.cpp

- * Calculate GPIO ports/pins number statically from devicetree configuration
- */
+#define DEVICE_GPIO(n, p, i) DEVICE_DT_GET(DT_PROP_BY_IDX(n, p, i))
+constexpr const struct device *gpios[] = {


The gpios array is declared constexpr but contains pointers to device structures obtained via DEVICE_DT_GET. In Zephyr, device pointers typically point to runtime-initialized structures and may not be valid in constexpr context. This could cause compilation errors depending on the Zephyr version and compiler. Consider using const instead of constexpr for the gpios array if compilation issues arise.

Suggested change

constexpr const struct device *gpios[] = {

const struct device *const gpios[] = {

soburi changed the title ~~zephyrCommon: Improved interrupt handling~~ [DNM] zephyrCommon: Improved interrupt handling Aug 23, 2025

soburi force-pushed the improve_interrupt branch from 3316f82 to 28b1df9 Compare August 30, 2025 03:19

soburi requested review from DhruvaG2000, beriberikix and szczys as code owners August 30, 2025 03:19

soburi force-pushed the improve_interrupt branch 4 times, most recently from ffc4797 to 5e8e11a Compare August 31, 2025 00:26

soburi force-pushed the improve_interrupt branch from 5e8e11a to a3f549f Compare September 10, 2025 07:41

soburi force-pushed the improve_interrupt branch from a3f549f to b398bfa Compare September 27, 2025 04:31

soburi added 3 commits December 19, 2025 23:21

Copilot AI review requested due to automatic review settings December 19, 2025 14:22

soburi force-pushed the improve_interrupt branch from b398bfa to 7304eac Compare December 19, 2025 14:22

Copilot started reviewing on behalf of soburi December 19, 2025 14:22 View session

Copilot AI reviewed Dec 19, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[DNM] zephyrCommon: Improved interrupt handling #136

[DNM] zephyrCommon: Improved interrupt handling #136

Uh oh!

soburi commented Aug 23, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Dec 19, 2025

Uh oh!

Copilot AI Dec 19, 2025

Uh oh!

Copilot AI Dec 19, 2025

Uh oh!

Copilot AI Dec 19, 2025

Uh oh!

Copilot AI Dec 19, 2025

Uh oh!

Copilot AI Dec 19, 2025

Uh oh!

Copilot AI Dec 19, 2025

Uh oh!

Copilot AI Dec 19, 2025

Uh oh!

Copilot AI Dec 19, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

	void handle_gpio_callback(const struct device port, struct gpio_callback cb, uint32_t pins) {
	void handle_gpio_callback(const struct device port, struct gpio_callback cb, uint32_t pins) {
	(void)port;

		struct gpio_port_callback *pcb = find_gpio_port_callback(local_gpio_port(pinNumber));

-  struct gpio_port_callback *pcb = find_gpio_port_callback(local_gpio_port(pinNumber));
+  const struct device *dev = local_gpio_port(pinNumber);
+  if (!dev) {
+    return;
+  }
+  struct gpio_port_callback *pcb = find_gpio_port_callback(dev);

		gpio_pin_interrupt_configure(local_gpio_port(pinNumber), local_gpio_pin(pinNumber), 0);
		disableInterrupt(pinNumber);

	constexpr const struct device *gpios[] = {
	const struct device *const gpios[] = {

[DNM] zephyrCommon: Improved interrupt handling #136

Are you sure you want to change the base?

[DNM] zephyrCommon: Improved interrupt handling #136

Uh oh!

Conversation

soburi commented Aug 23, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant